Skip to content

Avoid shutdown recursion#816

Merged
liamhuber merged 7 commits intomainfrom
shutdown-recursion
Mar 3, 2026
Merged

Avoid shutdown recursion#816
liamhuber merged 7 commits intomainfrom
shutdown-recursion

Conversation

@liamhuber
Copy link
Member

@liamhuber liamhuber commented Mar 2, 2026

In executorlib-1.8.2, which cancels futures when the executor is .shutdown.

This gets us into a recursive pickle when the _finish_run callback internall shuts down the executor. Passing that shutdown over to a separate, patient thread resolves all the issues in the local tests for executorlib-1.8.2. I need the CI to see if it also resolves the issue running with SLURM

@github-actions
Copy link

github-actions bot commented Mar 2, 2026

Binder 👈 Launch a binder notebook on branch pyiron/pyiron_workflow/shutdown-recursion

liamhuber added 2 commits March 2, 2026 12:16
@liamhuber
Bump executorlib to 1.8.2
efd2fa8 
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
@liamhuber
Avoid callback recursion
38009e1 
On executor shutdown

Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
@liamhuber liamhuber force-pushed the shutdown-recursion branch from 77edb16 to 38009e1 Compare March 2, 2026 20:16
@jan-janssen
Copy link
Member

jan-janssen commented Mar 2, 2026 •
edited
Loading

@liamhuber This should also work fine for 1.9.0 - as tested in #812

@liamhuber
Copy link
Member Author

liamhuber commented Mar 2, 2026

@liamhuber This should also work fine for 1.9.0

That's my hope. I just checked it out locally and am re-running the tests (sans SLURM), but it is not yet going nicely...

@liamhuber
Bump executorlib
c654e9b 
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
@liamhuber
Copy link
Member Author

liamhuber commented Mar 2, 2026

Yeah, locally I'm getting ResourceWarning: Enable tracemalloc to get the object allocation traceback and the cache test just hangs. Aha, and same thing on CI https://github.com/pyiron/pyiron_workflow/actions/runs/22594515927/job/65460954301?pr=817#step:4:1909

@codecov
Copy link

codecov bot commented Mar 2, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.36%. Comparing base (7035b23) to head (05bd76f).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
pyiron_workflow/mixin/run.py 50.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (50.00%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #816      +/-   ##
==========================================
- Coverage   93.46%   93.36%   -0.11%     
==========================================
  Files          38       38              
  Lines        3873     3873              
==========================================
- Hits         3620     3616       -4     
- Misses        253      257       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This was referenced Mar 2, 2026
Closed
Closed
@liamhuber liamhuber merged commit b37ca27 into main Mar 3, 2026
21 of 22 checks passed
@liamhuber liamhuber deleted the shutdown-recursion branch March 3, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@liamhuber @jan-janssen @pyiron-runner